PR progress checklist (to be filled in by reviewers)

• Changes to documentation are appropriate (or tick if not required)
• Changes to tests are appropriate (or tick if not required)
• Reviews completed

What type of PR is this?

Primary type

• [build] Changes related to the build system
• [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
• [ci] Changes to the continuous integration configuration
• [feat] A new feature
• [fix] A bug fix
• [perf] A code change that improves performance
• [refactor] A code change that neither fixes a bug nor adds a feature
• [revert] A change used to revert a previous commit
• [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

• [docs] Documentation changes
• [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

Sort of - any users that were populating firewalld service xml from their pillar will just need to prepend a 'lookup' key one level up from services.

Example of previous pillar data:

firewalld:
  services:
    sshcustom:
      short: sshcustom
      description: ....

New proposed pillar data with same functionality:

firewalld:
  lookup:
    services:
      sshcustom:
        short: sshcustom
        description: ....

The pillar.example file has been updated to reflect this new style.
However, since the filrewalld/services file.directory is not being managed with the clean=true option, this change won't delete any existing firewalld service xml on an existing minion but will just cease pushing updates until the pillar and/or defaults.yaml is corrected.

Related issues and/or pull requests

Describe the changes you're proposing

Instead of configuring firewalld service xml exclusively from pillar data, this change allows users to specify them in defaults.yaml. This can help reduce pillar data size for minions, but still fully supports users doing service customizations and merges/overrides simply by moving their service pillar data underneath the 'firewalld:lookup' key. An empty dictionary called 'services' has also been added to the stock defaults.yaml file

Pillar / config required to test the proposed changes

Taken from pillar.example :

firewalld:
  lookup:
    services:
      sshcustom:
        short: sshcustom
        description: >-
          SSH on port 3232 and 5252. Secure Shell (SSH) is a protocol for logging
          into and executing commands on remote machines. It provides secure
          encrypted communications. If you plan on accessing your machine
          remotely via SSH over a firewalled interface, enable this option. You
          need the openssh-server package installed for this option to be useful.
        ports:
          tcp:
            - 3232
            - 5252
        modules:
          - some_module_to_load
        protocols:
          - igmp
        source_ports:
          tcp:
            - 21
        destinations:
          ipv4:
            - 224.0.0.251
            - 224.0.0.252
          ipv6:
            - ff02::fb
            - ff02::fc

      zabbixcustom:
        short: Zabbixcustom
        description: "zabbix custom rule"
        ports:
          tcp:
            - "10051"
      salt-minion:
        short: salt-minion
        description: "salt-minion"
        ports:
          tcp:
            - "8000"

Debug log showing how the proposed changes work

The underlying functionality of the states are very preserved, just that services.sls loops over firewalld.services instead of salt['pillar.get']('firewalld:services')

Documentation checklist

• Updated the README (e.g. Available states).
• Updated pillar.example.

Testing checklist

• Included in Kitchen (i.e. under state_top).
• Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
• Updated the relevant test pillar.

Additional context